fix(rbac): block preview public channels - #2713
Conversation
📝 WalkthroughWalkthroughPublic channel creation and private-to-public promotion now require ChangesPublic channel authorization
Onboarding metric typing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ChannelRequest
participant ChannelsRLS
participant PublicTransitionTrigger
participant ChannelsTable
ChannelRequest->>ChannelsRLS: Insert or update public channel
ChannelsRLS->>PublicTransitionTrigger: Check private-to-public transition
PublicTransitionTrigger->>ChannelsTable: Allow or reject mutation
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_577ec601-13f5-4795-991d-aa88a224e618) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fa0907f3-988c-4aab-8f1d-9a7834fd8632) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ab96f4b2-26d5-4bc9-ba2d-e0ed44955c91) |
|
@cubic-dev-ai review |
@riderx I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_68f59008-fe23-4ba5-9dba-bae8401d1f42) |
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Keep channel UPDATE RLS channel-scoped so already-public edits still work. Require app.update_settings only on private-to-public transitions via a BEFORE UPDATE trigger, and align SQL/CLI isolation tests with that boundary. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_dd9f9b78-2dc7-49dc-b381-63e9c1061da4) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f6c23623-ff23-4326-80b3-3095151d9d8e) |
…rics Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e05c72e1-55c6-4eff-9ff4-f71c82244940) |
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f500015d-0402-46a4-87df-248606ea9bef) |
Match the UPDATE trigger: retain already-public channels with channel.update_settings alone. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f4e7bf33-9d73-4f46-8636-f862228bc4e9) |
Assert app_preview cannot create public channels, channel-admins cannot flip private to public, and already-public channel edits stay allowed. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_3ad78ede-614c-4f16-9b86-1051f42596d9) |
Use current_request_role()/is_internal_request_role() so anon API-key and pgTAP callers are not skipped when session_user is postgres. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_db8f1c76-6f86-4b9d-9ce0-d229d5dd9799) |
|
Fixed in |
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
supabase/migrations/20260808175552_app_preview_public_channel_guard.sql (1)
62-79: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftComplete the required execution-plan validation before merge.
This row-level trigger runs once per affected
public.channelsrow and callsrbac_check_permission_requestduring private-to-public transitions. Document execution frequency, roles, worst-case cardinalities, and indexes. RunEXPLAIN (ANALYZE, BUFFERS)against production-scale data and record the result.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260808175552_app_preview_public_channel_guard.sql` around lines 62 - 79, Complete the execution-plan validation for enforce_public_channel_app_settings_permission and its rbac_check_permission_request call: document per-row trigger frequency, involved roles, worst-case cardinalities, and required indexes; run EXPLAIN (ANALYZE, BUFFERS) using production-scale data, and record the resulting plan and performance findings with the migration or associated review documentation.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@supabase/migrations/20260808175552_app_preview_public_channel_guard.sql`:
- Around line 62-79: Complete the execution-plan validation for
enforce_public_channel_app_settings_permission and its
rbac_check_permission_request call: document per-row trigger frequency, involved
roles, worst-case cardinalities, and required indexes; run EXPLAIN (ANALYZE,
BUFFERS) using production-scale data, and record the resulting plan and
performance findings with the migration or associated review documentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7006d6ea-08dc-4e2a-8202-be2aa86fd7b3
📒 Files selected for processing (2)
supabase/migrations/20260808175552_app_preview_public_channel_guard.sqlsupabase/tests/67_test_public_channel_guard.sql
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_46c52df9-9a47-40cc-9af1-a0f02e5760fb) |
|
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Confidence score: 4/5
- In
supabase/tests/67_test_public_channel_guard.sql, the private→public update path is not exercised with the App Preview key, so the PR’s stated App Preview denial behavior could regress unnoticed and allow unintended channel promotion in that context—add a dedicated App Preview-key case on the UPDATE transition to close the coverage gap.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/tests/67_test_public_channel_guard.sql">
<violation number="1" location="supabase/tests/67_test_public_channel_guard.sql:265">
P3: The PR explicitly lists "App Preview ... can no longer create or promote public/default channels" and "App Preview denial" as core coverage, but this test never exercises the App Preview key on the private→public UPDATE-promotion path. Test 3 uses the channel-admin key (lacking app.update_settings) to prove the BEFORE trigger denies promotion, so the trigger denial is covered in effect, but there is no direct assertion that the app_preview key itself is denied on an UPDATE flip — only the INSERT denial (test 1). Consider adding a preview-key UPDATE-public-flip throws_ok to match the stated coverage.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| 'app_preview key can insert a private channel' | ||
| ); | ||
|
|
||
| -- 3) Channel-admin cannot flip private -> public. |
There was a problem hiding this comment.
P3: The PR explicitly lists "App Preview ... can no longer create or promote public/default channels" and "App Preview denial" as core coverage, but this test never exercises the App Preview key on the private→public UPDATE-promotion path. Test 3 uses the channel-admin key (lacking app.update_settings) to prove the BEFORE trigger denies promotion, so the trigger denial is covered in effect, but there is no direct assertion that the app_preview key itself is denied on an UPDATE flip — only the INSERT denial (test 1). Consider adding a preview-key UPDATE-public-flip throws_ok to match the stated coverage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/tests/67_test_public_channel_guard.sql, line 265:
<comment>The PR explicitly lists "App Preview ... can no longer create or promote public/default channels" and "App Preview denial" as core coverage, but this test never exercises the App Preview key on the private→public UPDATE-promotion path. Test 3 uses the channel-admin key (lacking app.update_settings) to prove the BEFORE trigger denies promotion, so the trigger denial is covered in effect, but there is no direct assertion that the app_preview key itself is denied on an UPDATE flip — only the INSERT denial (test 1). Consider adding a preview-key UPDATE-public-flip throws_ok to match the stated coverage.</comment>
<file context>
@@ -0,0 +1,323 @@
+ 'app_preview key can insert a private channel'
+);
+
+-- 3) Channel-admin cannot flip private -> public.
+SELECT tests.clear_authentication();
+SELECT set_config('request.jwt.claim.role', 'anon', true);
</file context>
2 participants
Summary
app.update_settingsto create a public/default channel.RLS execution model
public.channelsINSERTWITH CHECK; it runs once per inserted channel row from the CLI/PostgREST path.app.create_channelremains required for every new channel.public = trueadditionally requires the existing, caller-scopedapp.update_settingsRBAC check on the row owner app. App Preview does not receive that permission.owner_organdapp_idvalues.app.update_settingsguard before inserting.EXPLAIN and integration follow-up
EXPLAIN (ANALYZE, BUFFERS)and the affected lifecycle integration test are pending: the local Supabase runtime cannot start because Docker/Dory is unavailable (Cannot connect to .../.dory/dory.sock).Validation
bun lintbun lint:backendbun run typecheck:backendbunx vitest run tests/channel-post.unit.test.ts- 16 passedbun test:unit- 160 files / 1,065 tests passedNote
Medium Risk
Changes authorization for public channel creation and promotion across RLS, triggers, and the API—delivery-sensitive RBAC—but behavior is narrowly scoped with broad test coverage.
Overview
App Preview and channel-scoped keys can no longer create or promote public/default channels without
app.update_settings. Private preview channels and normal channel settings updates stay allowed under existing permissions.Database RLS on
channelsINSERT now requiresapp.update_settingswhenpublicis true, while UPDATE policies remain channel-scoped so admins can still edit already-public channels. A BEFORE UPDATE trigger onpublicenforces the same rule for private → public flips on direct PostgREST/CLI writes.The
POST /channelhandler loads existingpublicstate and applies the same publicizing check before create/update. pgTAP, unit, and CLI integration tests cover preview denial, channel-admin promotion denial, and unchanged edits on public channels.Reviewed by Cursor Bugbot for commit 3684d36. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes